Misra standard demand a single point of exit for a function, but I have the following "conversion" code
typedef enum { CASE_A, CASE_B, CASE_C } my_enum_t;
int my_conv_funct(my_enum_t value)
{
switch(value)
{
case CASE_A:
return 0;
case CASE_B:
return 1;
case CASE_C:
return 2;
default:
break;
}
log_error("ERROR!!!!!");
assert(1==0);
}
Is this valid? I need to convert it to a single return function? And what is the best way of handling the default case?
this creates an unreachable code in theory (the error is to warn in case one add a value in the enum and not add a corresponding case)
This is an embedded system btw having those asserts create issues?
Thanks, Nick
EDITED:
the default case should be never called if there are no errors (for example a programmer add another value in the enum and doesn't add a corresponding case
another options would be to remove the default at all but that violates another misra rule
typedef enum { CASE_A, CASE_B, CASE_C } my_enum_t;
int my_conv_funct(my_enum_t value)
{
switch(value)
{
case CASE_A:
return 0;
case CASE_B:
return 1;
case CASE_C:
return 2;
}
//should never reach this line
assert(1==0);
}
This will generate a warning if I compile and don't specify all the cases in the enum (I think)
Very simply:
int my_conv_funct(my_enum_t value)
{
int result = -1;
switch(value)
{
case CASE_A:
result = 0;
break;
case CASE_B:
result = 1;
break;
case CASE_C:
result = 2;
break;
default:
break;
}
if(result == -1)
{
log_error("ERROR!!!!!");
assert(1==0);
}
return result;
}
Is this valid?
It does not comply with the MISRA rule you described.
I need to convert it to a single return function?
To comply with the MISRA rule, yes.
And what is the best way of handling the default case?
We cannot judge what is "best" for your particular circumstances and use.
This is an embedded system btw having those asserts create issues?
The idea of an assertion is that it helps you find programming errors during development, but (in principle) it gets disabled via build options in code that is intended to be used in production. If that model is followed then the assertion itself probably does not create an issue, but the fact that the function does not return a value in the default case (if assertions are disabled) does. If the program must terminate in the event that the default case is exercised then it should call abort(), or some other function having that effect. Otherwise, it should return a sensible value in the default case.
I would probably write the function more like this:
int my_conv_funct(my_enum_t value)
{
switch(value)
{
case CASE_A:
case CASE_B:
case CASE_C:
break;
default:
log_error("ERROR!!!!!");
assert(0);
break;
}
return value;
}
There is now just one exit point from the function, and if it returns at all then it returns its argument (implicitly converted to type int).
First of all please check this answer: Best practice for compute the function return value. The MISRA-C rule is advisory and I recommend to make a permanent deviation against it. Personally I replace it with a rule such as:
"Multiple return statements in a function should be avoided unless they make the code more readable/maintainable."
The rationale to avoid returning from multiple places inside nested, complex code is sound, but far less so in clean and readable functions.
In your specific case though, I would perhaps have rewritten the function like this (MISRA compliant without ignoring the rule):
uint32_t my_conv_funct (my_enum_t value)
{
uint32_t result;
switch(value)
{
case CASE_A: result = 0; break;
case CASE_B: result = 1; break;
case CASE_C: result = 2; break;
default:
{
// error handling here
}
}
return result;
}
Alternatively (deviating from the rule):
uint32_t my_conv_funct (my_enum_t value)
{
static const uint32_t lut[] = { CASE_A, CASE_B, CASE_C };
for(size_t i=0; i<sizeof lut/sizeof *lut; i++)
{
if(lut[i] == value)
{
return i;
}
}
/* error handling */
return some_error_code;
}
This assuming that the amount of items isn't large, in which case a binary search might be more inefficient.
This in turn assuming that the enum constants don't correspond to 0, 1 and 2 in which case the whole function is nonsense.